[RACL] Implement config parsing, racl_ctrl, and reggen checks #25664
Conversation
Razer6
requested review from
andreaskurth,
rswarbrick and
matutem
and removed request for
msfschaffner
Razer6
force-pushed
the
racl-package
branch
7 times, most recently
from
d3da1a9 to
fecd82b
Compare
Razer6
force-pushed
the
racl-package
branch
2 times, most recently
from
7d1df47 to
dfcda08
Compare
|
@andreaskurth Thanks for the initial feedback. I changed the docs or created issues for it. Can you take another look? |
Razer6
changed the title
Razer6
force-pushed
the
racl-package
branch
4 times, most recently
from
ade3d52 to
465f2af
Compare
| def generate_racl(topcfg: Dict[str, object], out_path: Path) -> None: | ||
| # Not all tops use RACL | ||
| if 'racl_config' not in topcfg: | ||
| return |
There was a problem hiding this comment.
Wouldn't it make sense to set topcfg['racl'] to None in this situation?
There was a problem hiding this comment.
The reason this was not added is to make it easier to detect if no RACL is used, i.e., down below: completecfg.get('racl', DEFAULT_RACL_CONFIG)
There was a problem hiding this comment.
Detection of no RACL could be as simple as racl not in completecfg, and we could have no trace of racl if it is not used. Perhaps the package could be avoided in those cases to keep it simple.
| @@ -115,7 +117,13 @@ | |||
| %> | |||
| `include "prim_assert.sv" | |||
|
|
|||
| module ${mod_name} ( | |||
| module ${mod_name}${' (' if not racl_support else ''} | |||
There was a problem hiding this comment.
To avoid the isolated # stuff, maybe it makes sense to use something like ${' # (' if racl_support else ' ('} ?
There was a problem hiding this comment.
The only reason for that was to keep the diff 0 when not enabling RACL on one IP. Otherwise we would generate a small whitespace diff for all regtops.
Signed-off-by: Robert Schilling <[email protected]>
Signed-off-by: Robert Schilling <[email protected]>
Signed-off-by: Robert Schilling <[email protected]>
| } | ||
|
|
||
|
|
||
| def parse_racl_config(config_path: str): |
There was a problem hiding this comment.
Coud you add a return type, probably ...str) -> Dict[str, object]: or some more specific type for the value in the Dict?
| except OSError: | ||
| raise SystemExit(sys.exc_info()[1]) | ||
|
|
||
| # TODO further error handling |
There was a problem hiding this comment.
Some hint about the missing checks?
|
|
||
| for racl_group, policies in racl_config['policies'].items(): | ||
| for policy in policies: | ||
| def compute_policy_value(permission: str): |
There was a problem hiding this comment.
Seems like the return value type delaration could be ... str) -> int:
|
It was discussed the we always create the ports and tie them off at reg_top level |
This is the initial PR implementing RACL and consists of three commits:
Parse the RACL configuration file that is passed int the top-level HJSON file and render the
top_racl_pkg.svfrom it. If no RACL is used in the top, i.e., there is no RACL config passed, a defaulttop_racl_pkg.svis rendered to provide the minimal type information for all RACL usages in IPs. This package is a virtual core, and different Tops can generate their own version.Implement the ipgen'ed
racl_ctrlpolicy distributing IP. This IP renders all policies of a given policy group that is defined in theracl.hjsonfile. Currently, topgen only supports rendering the first RACL group'sracl_ctrlIP due to some tooling limitations. Note,racl_ctrl's registers themselves are not yet protected with RACL.Implement reggen support for RACL checks. On IP.hjson level, you can enable RACL on a per-bus granularity.